Skip to content

Conversation

@skunert
Copy link
Contributor

@skunert skunert commented Sep 30, 2025

Before this change we did not care about parachain slots in manual-seal mode, because para slots that are smaller than their relay parents where allowed. I have changes planned that change this, so the manual-seal needs to be parameterized closer to reality.

In this PR I use the AuraConsensusDataprovider to read the inherent timestamp provided and emit a corresponding slot digest. I also removed the DynNodeSpecExt and added manual seal methods directly to the NodeSpec trait. The assumption before was that the manual seal mode can work independently of consensus information. But this no longer holds true, as we need the proper AuraId and runtime APIs attached for example. This was the fastest and simplest way to get this working, and we currently only support AuraNode anyway.

closes #7453

Prerequisite for mergine #9712, as the old way we did things now causes errors.

@skunert skunert added this to SDK Node Sep 30, 2025
@skunert skunert added the T0-node This PR/Issue is related to the topic “node”. label Sep 30, 2025
@github-project-automation github-project-automation bot moved this to backlog in SDK Node Sep 30, 2025
@skunert
Copy link
Contributor Author

skunert commented Sep 30, 2025

/cmd prdoc --audience runtime_dev runtime_user --bump major --force

# Conflicts:
#	cumulus/polkadot-omni-node/lib/src/nodes/aura.rs
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18161369981
Failed job name: fmt

@skunert
Copy link
Contributor Author

skunert commented Oct 1, 2025

/cmd fmt

@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2025

Command "fmt" has failed ❌! See logs here

@skunert skunert requested a review from lexnv October 6, 2025 07:01
block_announce_validator_builder: None,
warp_sync_config: None,
block_relay: None,
metrics: NotificationMetrics::new(None),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dq: We are not using the config.prometheus_metrics because we don't want to expose extra backend metrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual-seal node is just for local dev, to get some blocks out and test your runtime. We could skip the entire instantiation of the networking IMO, since the blocks are not supposed to sync anywhere. But yeah, some of the components lower down expect network currently, so I am instantiating it but keeping everything as slim as possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oki doki makes perfect sense, thanks!

@skunert skunert enabled auto-merge October 6, 2025 08:44
@skunert skunert added this pull request to the merge queue Oct 6, 2025
Merged via the queue into master with commit 0d22db5 Oct 6, 2025
279 of 284 checks passed
@skunert skunert deleted the skunert/manual-seal-proper-aura branch October 6, 2025 14:22
@github-project-automation github-project-automation bot moved this from backlog to done in SDK Node Oct 6, 2025
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
Before this change we did not care about parachain slots in manual-seal
mode, because para slots that are smaller than their relay parents where
allowed. I have changes planned that change this, so the manual-seal
needs to be parameterized closer to reality.

In this PR I use the `AuraConsensusDataprovider` to read the inherent
timestamp provided and emit a corresponding slot digest. I also removed
the `DynNodeSpecExt` and added manual seal methods directly to the
`NodeSpec` trait. The assumption before was that the manual seal mode
can work independently of consensus information. But this no longer
holds true, as we need the proper `AuraId` and runtime APIs attached for
example. This was the fastest and simplest way to get this working, and
we currently only support `AuraNode` anyway.

closes #7453 

Prerequisite for mergine #9712, as the old way we did things now causes
errors.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T0-node This PR/Issue is related to the topic “node”.

Projects

Status: done

Development

Successfully merging this pull request may close these issues.

omni-node: Improve manual-seal parameterization

4 participants